Derive total_byte_size for join statistics by Dandandan · Pull Request #24536 · apache/datafusion

Dandandan · 2026-08-20T19:48:56Z

Which issue does this PR close?

Split out of feat: cost-based join order enumeration #24456, which needs this to cost joins but does not depend on the rest of that PR.

Rationale for this change

Currently it returns total_byte_size: Precision::Absent` for most joins - we can derive it to produce better estimates for nested joins.

What changes are included in this PR?

The width of one output row follows from the sides a join emits, which the join type
already says: both sides for an inner or outer join, the preserved side for a semi or
anti join, and one side plus a boolean for a mark join. Multiplied by the estimated
cardinality, that gives the output size.

Are these changes tested?

Yes. A new unit test covers an inner join summing both sides, a mark join adding one
bit per row, and a semi join keeping its column-derived size.

Benchmarks

TPC-DS SF1: three plans change, each because a join that
now reports a size clears hash_join_single_partition_threshold and becomes
CollectLeft.

query	before	after
ds-q10	55.0 ms	49.8 ms	0.905x
ds-q35	62.1 ms	56.3 ms	0.907x
ds-q91	37.9 ms	37.0 ms	0.976x

No query regressed.

Are there any user-facing changes?

No API change. Joins now report a total_byte_size where they previously reported
none, which EXPLAIN ANALYZE and statistics-driven decisions can read.

A join knew how many rows it would emit but not how wide they were: every join type but semi and anti reported no total byte size at all. An operator above it reading that size finds nothing, so `hash_join_single_partition_threshold`, a byte threshold, falls back to counting rows. The width of an output row follows from the sides the join emits, which the join type already says: both sides for an inner or outer join, the preserved side for a semi or anti join, and one side plus a boolean for a mark join. Multiplying it by the estimated cardinality gives the size. Semi and anti joins keep the size they derive from their column statistics, which knows which columns survive; the width fills a gap rather than replacing a better answer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YiAABcW4WSqij31zz2P6c

codecov-commenter · 2026-08-20T20:11:24Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.32%. Comparing base (62650ef) to head (8bf6b3f).
⚠️ Report is 249 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #24536      +/-   ##
==========================================
+ Coverage   80.87%   81.32%   +0.44%     
==========================================
  Files        1101     1117      +16     
  Lines      375765   396322   +20557     
  Branches   375765   396322   +20557     
==========================================
+ Hits       303915   322299   +18384     
- Misses      53747    55191    +1444     
- Partials    18103    18832     +729

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Dandandan · 2026-08-20T20:32:50Z

run benchmarks

adriangbot · 2026-08-20T20:34:57Z

Benchmark for this request failed before finishing (Kubernetes reason: BackoffLimitExceeded).

Benchmarks requested: clickbench_partitioned

Kubernetes message

Job has reached the specified backoff limit

File an issue against this benchmark runner

adriangbot · 2026-08-20T20:35:37Z

Benchmark for this request failed before finishing (Kubernetes reason: BackoffLimitExceeded).

Benchmarks requested: tpch

Kubernetes message

Job has reached the specified backoff limit

File an issue against this benchmark runner

adriangbot · 2026-08-20T20:35:44Z

Benchmark for this request failed before finishing (Kubernetes reason: BackoffLimitExceeded).

Benchmarks requested: tpcds

Kubernetes message

Job has reached the specified backoff limit

File an issue against this benchmark runner

Dandandan · 2026-08-20T20:42:12Z

run benchmarks

Dandandan · 2026-08-20T20:44:06Z

run benchmark tpc-ds

Dandandan · 2026-08-20T20:44:19Z

run benchmark tpcds

adriangbot · 2026-08-20T20:44:52Z

Benchmark for this request failed before finishing (Kubernetes reason: BackoffLimitExceeded).

Benchmarks requested: tpcds

Kubernetes message

Job has reached the specified backoff limit

File an issue against this benchmark runner

adriangbot · 2026-08-20T20:44:59Z

Benchmark for this request failed before finishing (Kubernetes reason: BackoffLimitExceeded).

Benchmarks requested: clickbench_partitioned

Kubernetes message

Job has reached the specified backoff limit

File an issue against this benchmark runner

adriangbot · 2026-08-20T20:45:03Z

Benchmark for this request failed before finishing (Kubernetes reason: BackoffLimitExceeded).

Benchmarks requested: tpch

Kubernetes message

Job has reached the specified backoff limit

File an issue against this benchmark runner

adriangbot · 2026-08-20T20:45:46Z

Benchmark for this request failed before finishing (Kubernetes reason: BackoffLimitExceeded).

Benchmarks requested: tpcds

Kubernetes message

Job has reached the specified backoff limit

File an issue against this benchmark runner

adriangbot · 2026-08-20T20:46:35Z

Benchmark for this request failed before finishing (Kubernetes reason: BackoffLimitExceeded).

Benchmarks requested: tpc-ds

Kubernetes message

Job has reached the specified backoff limit

File an issue against this benchmark runner

Dandandan · 2026-08-21T06:10:37Z

run benchmark tpcds

adriangbot · 2026-08-21T06:14:03Z

Benchmark for this request failed before finishing (Kubernetes reason: BackoffLimitExceeded).

Benchmarks requested: tpcds

Kubernetes message

Job has reached the specified backoff limit

File an issue against this benchmark runner

asolimando

LGTM, a few non-blocking comments. As suggested elsewhere, I'd be curious to see if #23975 can measure any improvement for this

asolimando · 2026-08-21T16:59:01Z

This is stale now

asolimando · 2026-08-21T17:00:38Z

+                // Only fill a gap: a size the join derived from its column statistics
+                // knows which columns it keeps, which an average row width cannot.
+                (Precision::Absent, Some(width)) => {
+                    Precision::Inexact((stats.num_rows as f64 * width) as usize)


Minor: maybe we could round here instead of truncating?

Dandandan · 2026-08-21T17:09:51Z

run benchmark tpch tpcds tpch10

adriangbot · 2026-08-21T17:13:19Z

Benchmark for this request failed before finishing (Kubernetes reason: BackoffLimitExceeded).

Benchmarks requested: tpch

Kubernetes message

Job has reached the specified backoff limit

File an issue against this benchmark runner

adriangbot · 2026-08-21T17:13:20Z

Benchmark for this request failed before finishing (Kubernetes reason: BackoffLimitExceeded).

Benchmarks requested: tpcds

Kubernetes message

Job has reached the specified backoff limit

File an issue against this benchmark runner

adriangbot · 2026-08-21T17:13:20Z

Benchmark for this request failed before finishing (Kubernetes reason: BackoffLimitExceeded).

Benchmarks requested: tpch10

Kubernetes message

Job has reached the specified backoff limit

File an issue against this benchmark runner

Dandandan · 2026-08-22T21:22:04Z

run benchmark tpcds tpch10

adriangbot · 2026-08-22T21:24:33Z

Benchmark for this request failed before finishing (Kubernetes reason: BackoffLimitExceeded).

Benchmarks requested: tpcds

Kubernetes message

Job has reached the specified backoff limit

File an issue against this benchmark runner

adriangbot · 2026-08-22T21:24:37Z

Benchmark for this request failed before finishing (Kubernetes reason: BackoffLimitExceeded).

Benchmarks requested: tpch10

Kubernetes message

Job has reached the specified backoff limit

File an issue against this benchmark runner

viirya

Thanks for splitting this out — deriving a size instead of reporting Absent is clearly worth having, and the benchmark table showing three TPC-DS plans improving with no regressions is convincing. A few questions inline.

viirya · 2026-08-23T16:37:41Z

 ) -> Result<Statistics> {
+    // Width of one output row, from the sides this join emits. Without it a join reports
+    // no size and `hash_join_single_partition_threshold` falls back to counting rows.
+    let width = |stats: &Statistics| match (


Why an f64 average row width rather than staying in Precision? stats_cartesian_product in cross_join.rs derives its output size with Precision arithmetic throughout (left.total_byte_size.multiply(&right_row_count).add(...)), which keeps integer precision and gets the Exact/Inexact lattice for free. Here the width round-trips through f64. Was Precision arithmetic considered and rejected — presumably because the per-side row-count multiplication that works for a cartesian product doesn't map onto an equi-join? If f64 is the right call, a brief comment saying why would help, since the neighbouring code sets a different precedent.

Also: reading through get_value() means the result is Inexact even when both sides are Exact. I assume that's deliberate (an average row width is inexact regardless), but worth confirming it's intent rather than an artifact.

viirya · 2026-08-23T16:37:41Z

+        _ => None,
+    };
+    // The boolean a mark join appends is one bit per row.
+    const MARK_COLUMN_WIDTH: f64 = 1.0 / 8.0;


1/8 byte matches Field::new("mark", DataType::Boolean, false) — non-nullable, so there's no validity buffer to account for. Noting I checked, in case that field ever becomes nullable and this constant needs revisiting.

viirya · 2026-08-23T16:37:41Z

+    // The boolean a mark join appends is one bit per row.
+    const MARK_COLUMN_WIDTH: f64 = 1.0 / 8.0;
+    let output_width = match join_type {
+        JoinType::LeftSemi | JoinType::LeftAnti => width(&left_stats),


Is this arm reachable in practice? It only applies when the cardinality estimate returned Absent for total_byte_size while the left input still has both total_byte_size and num_rows — and your semi-join test takes the column-derived path instead, so it doesn't exercise this branch. If the combination does occur (a source with a file-level size but no per-column byte_size, say), a test with left stats where total_byte_size is present but per-column byte_size is absent would pin the behaviour down.

viirya · 2026-08-23T16:37:41Z

+                // Only fill a gap: a size the join derived from its column statistics
+                // knows which columns it keeps, which an average row width cannot.
+                (Precision::Absent, Some(width)) => {
+                    Precision::Inexact((stats.num_rows as f64 * width) as usize)


Since total_byte_size feeds hash_join_single_partition_threshold in join_selection.rs, this estimate now decides whether a join becomes CollectLeft — which is exactly the effect your benchmarks show. For fixed-width columns the average row width is the true width, but for Utf8/List it can sit well below the widest rows. Have you considered whether an under-estimate here can flip a large build side into CollectLeft and raise memory use? If that's a real risk, rounding the derived size up — or only filling the gap for fixed-width schemas — might be worth considering.

github-actions Bot added core Core DataFusion crate physical-plan Changes to the physical-plan crate labels Aug 20, 2026

Dandandan requested a review from alamb August 20, 2026 20:41

Dandandan changed the title ~~fix: a join reports the size of the rows it emits~~ Add total_byte_size to join outputs Aug 20, 2026

Dandandan changed the title ~~Add total_byte_size to join outputs~~ Derive total_byte_size for join statistics Aug 20, 2026

Dandandan mentioned this pull request Aug 21, 2026

feat: cost-based join order enumeration #24456

Draft

asolimando approved these changes Aug 21, 2026

View reviewed changes

viirya reviewed Aug 23, 2026

View reviewed changes

Uh oh!

Conversation

Dandandan commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Benchmarks

Are there any user-facing changes?

Uh oh!

codecov-commenter commented Aug 20, 2026

Codecov Report

Uh oh!

Dandandan commented Aug 20, 2026

Uh oh!

adriangbot commented Aug 20, 2026

Uh oh!

adriangbot commented Aug 20, 2026

Uh oh!

adriangbot commented Aug 20, 2026

Uh oh!

Dandandan commented Aug 20, 2026

Uh oh!

Dandandan commented Aug 20, 2026

Uh oh!

Dandandan commented Aug 20, 2026

Uh oh!

adriangbot commented Aug 20, 2026

Uh oh!

adriangbot commented Aug 20, 2026

Uh oh!

adriangbot commented Aug 20, 2026

Uh oh!

adriangbot commented Aug 20, 2026

Uh oh!

adriangbot commented Aug 20, 2026

Uh oh!

Dandandan commented Aug 21, 2026

Uh oh!

adriangbot commented Aug 21, 2026

Uh oh!

asolimando left a comment

Choose a reason for hiding this comment

Uh oh!

asolimando Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

asolimando Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

Dandandan commented Aug 21, 2026

Uh oh!

adriangbot commented Aug 21, 2026

Uh oh!

adriangbot commented Aug 21, 2026

Uh oh!

adriangbot commented Aug 21, 2026

Uh oh!

Dandandan commented Aug 22, 2026

Uh oh!

adriangbot commented Aug 22, 2026

Uh oh!

adriangbot commented Aug 22, 2026

Uh oh!

viirya left a comment

Choose a reason for hiding this comment

Uh oh!

viirya Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

viirya Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

viirya Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

viirya Aug 23, 2026

Choose a reason for hiding this comment

Uh oh!

Dandandan commented Aug 20, 2026 •

edited

Loading